Skip to content

nmt: use bytes instead of list for send_message and send_periodic#674

Open
bizfsc wants to merge 3 commits into
masterfrom
feature/nmt-use-bytes
Open

nmt: use bytes instead of list for send_message and send_periodic#674
bizfsc wants to merge 3 commits into
masterfrom
feature/nmt-use-bytes

Conversation

@bizfsc

@bizfsc bizfsc commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

This PR extracts only the nmt.py changes from #651, which covers a broader set of mypy fixes across multiple files. Once the changes here are merged, #651 can be closed, as everything but the test_od changes are covered by other PRs.

The python-can send_message and send_periodic APIs expect bytes, not list[int]. Passing a list is accepted at runtime by some backends but is incorrect according to the type signatures and triggers Pylance and mypy errors. Using bytes literals and bytes() constructor makes the intent explicit and removes the type errors without any behavioral change.

bizfsc and others added 3 commits June 24, 2026 16:24
Use the CanData type alias provided by python-can to properly describe
the accepted types for message data.  This better matches the
parameter description in the docstring, which specifically does *not*
require a bytes-like object.
@acolomb

acolomb commented Jun 28, 2026

Copy link
Copy Markdown
Member

Why don't we just fix our type annotations on the sending functions? Added a commit to that effect. With the proper typing in our network module, none of these fixes are necessary. It aligns with the parameter docstring on send_message(), which already stated that "anything that can be converted to bytes" is acceptable. Since this actually works at runtime, too, the annotation was simply too narrow.

can.typechecking.CanData already gives us exactly the right combination of accepted types ready for use, and future-proof.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants